Repository navigation
Stream Redis-backed Spotify data and reserve homepage track space - #174
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
nwthomas
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 695c53ca11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const cachedTrack = await redis.get<NowPlayingTrack | null>(getSpotifyRecentlyPlayedRedisKey()); | ||
| if (cachedTrack) { | ||
| return cachedTrack; |
There was a problem hiding this comment.
In production, this cache lookup now occurs before confirming that the optional Spotify credentials are configured. If a deployment removes or incompletely configures those variables, an old listening record can continue to be published until the Redis TTL expires; with an empty cache, the disabled widget also performs a needless Redis operation. The previous ordering returned null before touching the cache, so add a local credential-presence check before this lookup without necessarily refreshing the access token.
AGENTS.md reference: AGENTS.md:L95-L97
Useful? React with 👍 / 👎.
| if (!res.ok) { | ||
| throw new Error(`Spotify request failed (${res.status})`); |
There was a problem hiding this comment.
When the refresh token grants user-read-recently-played but not the distinct user-read-currently-playing scope, this optional current-track request can return 403 even though the recently-played request remains authorized; Spotify documents the endpoints with separate required scopes (current track, recent tracks). Throwing here prevents getNowPlaying() from reaching its recently-played fallback, whereas the previous implementation treated a non-OK current-track response as a miss, so preserve that fallback for this case.
Useful? React with 👍 / 👎.
The homepage currently blocks its main content on Spotify and Redis. Move fetching and rendering into a single async SpotifyRecentlyPlayed component behind Suspense so the rest of
/streams while the lookup runs.Keep Redis as the only Spotify cache, retaining its existing one-hour TTL. Read Redis before requesting a Spotify token so cache hits avoid Spotify entirely. Add three-second timeouts per Spotify request and catch lookup failures inside the widget.
Reserve space for the heading and 48 px track row while loading. Limit track text to two lines to keep long titles from shifting content. Collapse the slot when the streamed result is empty or fails; collapsing an empty slot intentionally moves following content upward.
Validation:
These checks use stubs, not live Redis/Spotify credentials or production latency.